-
Notifications
You must be signed in to change notification settings - Fork 17
added bfs and operator<< methods #58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
include/GraphMatrix.hpp
Outdated
| } | ||
| } | ||
|
|
||
| friend std::ostream& operator<<(std::ostream& src, const GraphMatrix& other) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is there a << operator in the class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly for convenience. It allows for printing basic information about the graph using
std::cout << graph
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats a good approach.
But actually its not needed, the << operator overload is reserved for future to read graphs from a file, also user can print the graphs according to how they want, printing graphs on the console is not the concern of the library. 👍🏼
Update local repository with updated branch
include/GraphMatrix.hpp
Outdated
| } | ||
| } | ||
|
|
||
| std::vector<EdgeType> bfs(size_t start, size_t maxDepth) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the start variable be of type VertexType? because user is passing in a vertex to start bfs from?
| std::vector<EdgeType> bfs(size_t start, size_t maxDepth) { | |
| std::vector<EdgeType> bfs(VertexType start, size_t maxDepth) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you're rights. I can make that change
|
Your current implementation will not work with custom vertex and edge types. There is a reason that as of now the library doesn't have its While i appreciate taking your interest in contributing to Appledore, the algorithms implementation such as dfs and bfs will require much time and testing to be implmented. If you'd like you can work on the following open issues, or if you'd like to continue working with your
|
|
Ok, I will work on writing some tests and making the implementation work independent of type. Any feedback from time to time is much appreciated |
|



Additions: